Basic support for Jest runner #335

Kureev · 2017-10-11T12:12:13Z

With this PR merged in, users will be capable of using Jest test runner without significant problems:

Now it is possible to use standard interface to run tests:

detox test -r jest

instead of

jest e2e --setupTestFrameworkScriptFile=./jest/setup/e2e-tests.js --runInBand

Documentation reflects new test runner mechanism

rotemmiz

<3

rotemmiz · 2017-10-11T12:15:06Z

docs/Guide.Jest.md

    "test:e2e:build": "detox build"
 }
 ```

-We need the `--runInBand` as detox doesn't support parallelism yet.


Will running without the flag run tests serially?

I added the flag here so if you use detox test -r jest it is always there

What is stopping detox from parallelism?

a few things, mainly controlling multiple simulators. It's in our roadmap, shouldn't be too hard.

rotemmiz · 2017-10-11T12:16:12Z

detox/local-cli/detox-test.js

@@ -38,6 +38,9 @@ switch (program.runner) {
  case 'mocha':
    command = `node_modules/.bin/${program.runner} ${testFolder} --opts ${testFolder}/${program.runnerConfig} ${configuration} ${loglevel} ${cleanup} ${reuse} ${debugSynchronization} ${artifactsLocation}`;
    break;
+  case 'jest':


What about passing detox command line args? Will those work?

~~We can think of a way doing this. Although, in this version I wasn't aiming to provide this. If this is mandatory, we can discuss the approach~~

Sorry, misread your comment. What do you mean by "detox cli args"? Can you make an example?

I think it is, the feature is not 100% usable without it.

detox test --configuration ios.sim.release
detox test --loglevel verbose
detox test --debug-synchronization

Which flags are important?
In the previous version there were no recommendations regarding flags so kept it out of scope. However, I'm fine adding important flags to make jest support even better 💪 .

@rotemmiz what is the goal?

tests can access CLI arguments
or

detox can access CLI arguments

If we talk about the second one, then it already works like this (commander parses arguments before passing them to test runner)

You're right, but we execute a child process and these argument are not being passed into it.
cp.execSync(command, {stdio: 'inherit'});
We probably just need to pass them on...

Oh, now I see. Indeed, we just need to pass arguments to the child process. Then there will be no point in passing all these flags in the command itself

Seems like jest doesn't support allow custom CLI parameters. The only approach I see is to use environment vars.

Using ENV vars will imply changes on the mocha part and the way test cases gets parameters overall.

UPD: I can update argparse library and CLI so instead of additional flags it'll create one-time ENV variables. For user it'll be still the same, all mapping will happen under the hood.

Nice idea @Kureev, thank you so much for your efforts here 👍

DanielMSchmidt · 2017-10-11T17:46:26Z

@rotemmiz just a question, would you add different runner to the test matrix?

hellogerard · 2017-10-11T22:26:52Z

Totally tangential question, but here goes... Been playing around with detox and jest, and I noticed that if I have two test files (e.g. firstTest.spec.js and secondTest.spec.js), detox will terminate, uninstall, install and launch the app for each file. It does not do this with mocha. Any ideas? Is this just because of the repeated call to detox.init()? If so, where would a good place to put this so that it is only called once?

Edit: Happy to create a separate issue if you want.

Kureev · 2017-10-11T22:28:36Z

detox will terminate, uninstall, install and launch the app for each file.

@hellogerard I believe it is related to --reuse flag. See detox-test.js source for details

rotemmiz · 2017-10-11T22:49:28Z

I didn't use Detox with Jest yet, but from what I could see, there is no way to run a global before across files to create a one time setup (which is how detox.init() should be run), so this is indeed an issue.
reference: jestjs/jest#3832

rotemmiz · 2017-10-11T22:57:37Z

@DanielMSchmidt Not sure, the only thing we need to test here are is how the test runner is triggered, so it's pretty easy to test with unit tests.

hellogerard · 2017-10-12T04:14:03Z

reference: jestjs/jest#3832

@rotemmiz Thanks for that link. There's a PR that closes that issue. Once it's merged, it looks like it can solve the global setup issue. PR is jestjs/jest#4506

Kureev · 2017-10-13T08:09:38Z

Quick update on the PR: we discussed the way to go with @rotemmiz so you may expect that I'll push my changes today-tomorrow 😉

hellogerard · 2017-10-13T15:10:33Z

Hmm. I may have spoke too soon, or I don't understand the jest changes. I tried to use the new global setup by doing the following:

Created two test files (e.g. first.spec.js and second.spec.js).

Created a jest-environment-detox.js file:

require("babel-polyfill");

const NodeEnvironment = require('jest-environment-node');

const detox = require('detox');
const config = require('../package.json').detox;

class DetoxEnvironment extends NodeEnvironment {
  async setup() {
    try {
      await detox.init(config, { launchApp: true });
    } catch (e) {
      console.log('e', e);
    }
    await super.setup()
  }

  async teardown() {
    try {
      await detox.cleanup();
    } catch (e) {
      console.log('e', e);
    }
    await super.teardown()
  }
}

module.exports = DetoxEnvironment;

Specifying the test environment by passing --testEnvironment="./e2e/jest-environment-detox" to the jest command (full command: jest e2e --runInBand --forceExit --testEnvironment="./e2e/jest-environment-detox")

But no dice. Not only is it running detox.init for each test file, it's not even loading right (i.e. getting device is not defined). Am I doing this right? 😕

EDIT: Forgot to mention I did in fact install jest 21.3.0-beta.2.

Kureev · 2017-10-14T11:35:04Z

Hi @rotemmiz!

It seems like I have all my unit tests passing locally, but got some issues on CI 🤔 . Can you help me to figure why?

rotemmiz · 2017-10-14T16:19:13Z

Seems like some of the params passed to mocha have changed, try running scripts/ci.sh and see if it passes locally

Kureev · 2017-10-14T19:52:53Z

@rotemmiz seems that after the last fix CI dies due to timeout 😐 . Should I increase it a little bit?

rotemmiz · 2017-10-14T19:56:50Z

I have a feeling that something has changed in Travis. We’ll need to debug it, and add better visibility to the build log.

Does everything work locally?

Kureev · 2017-10-14T20:01:03Z

Does everything work locally?

I have the same problem with timeout on the local machine, but I'll try to extend it (locally) and let you know if it actually works.

Kureev · 2017-10-14T20:09:58Z

Ok, it gets stuck at

detox verb 3: /usr/bin/xcrun simctl terminate 33E49BFA-ED99-4ACD-A865-5E18863556B7 org.reactjs.native.example.example
detox info 3: Terminating org.reactjs.native.example.example...
detox info 3: org.reactjs.native.example.example terminated
detox verb 4: /bin/cat /dev/null >$HOME/Library/Developer/CoreSimulator/Devices/33E49BFA-ED99-4ACD-A865-5E18863556B7/data/tmp/detox.last_launch_app_log.out 2>$HOME/Library/Developer/CoreSimulator/Devices/33E49BFA-ED99-4ACD-A865-5E18863556B7/data/tmp/detox.last_launch_app_log.err && SIMCTL_CHILD_DYLD_INSERT_LIBRARIES="/Users/xamd/Library/Detox/ios/a719a7fd8af96db5fefad3da20b783f61b168620/Detox.framework/Detox" /usr/bin/xcrun simctl launch --stdout=/tmp/detox.last_launch_app_log.out --stderr=/tmp/detox.last_launch_app_log.err 33E49BFA-ED99-4ACD-A865-5E18863556B7 org.reactjs.native.example.example --args -detoxServer ws://localhost:57195 -detoxSessionId adf47c1c-c956-e93c-7640-cf174d884a54
detox info 4: Launching org.reactjs.native.example.example...

Even if I run my emulator manually, still the same issue... No idea how to debug it. Any suggestions?

UPD: Got an error after timeout

The request was denied by service delegate (SBMainWorkspace) for reason: NotFound ("Application "org.reactjs.native.example.example" is unknown to FrontBoard").

Not sure if it is anyhow related to my patch though 🤔

rotemmiz · 2017-10-14T20:13:41Z

I’m not near a computer to verify that, but I think that —args is misplaced (check the last command vs Detox 5.8.2)

Kureev · 2017-10-14T21:28:14Z

I found an issue in my local setup: seems for some reason my emulator that was used by tests got stuck (no idea, really). After resetting all the settings, everything worked. Will report once I finish all tests locally. 🤞

Kureev · 2017-10-14T21:38:42Z

I can confirm that all tests (including e2e) are green.

cc @rotemmiz

rotemmiz · 2017-10-14T21:43:15Z

Rerunning your build

rotemmiz · 2017-10-14T21:56:58Z

I want to add a very important request to this feature: since users choose and setup their test runner probably stick with it for more than an hour, it is important to add an option to configure the test runner in the detox config inside package.json (just like we are able configure the test folder), and ditch the test runner selection as a command line argument.

This should be a very small addition.

Kureev · 2017-10-14T22:29:53Z

Tested locally on both Xcode 8.3.3 and Xcode 9, could not reproduce the timeout we're having in CI. I'll have to check tomorrow.

If there is a way to reload your iOS emulator on Travis, I would do that first. For me it solved exactly the same problem on my local machine.

Kureev · 2017-10-15T22:33:21Z

Ready for review, @rotemmiz

rotemmiz · 2017-10-16T12:48:55Z

@Kureev, some of our users use Detox with mocha (or any other test runner for that matter) independently of detox-cli. The current change breaks their setup since they can not pass arguments anymore.
When we spoke, I encouraged you use argv and merge them with env vars if needed.
The fact that Jest doesn't support argv doesn't need to enforce other test runners to work with env vars.

Kureev · 2017-10-16T13:07:11Z

The current change breaks their setup since they can not pass arguments anymore.
When we spoke, I encouraged you use argv and merge them with env vars if needed.
The fact that Jest doesn't support argv doesn't need to enforce other test runners to work with env vars.

Not sure I follow. All arguments are respected 🤔

rotemmiz · 2017-10-16T13:10:53Z

Detox core now only supports env vars, not argv. Try running
node_modules/.bin/mocha e2e --opts e2e/mocha.opts --configuration ios.sim.release --loglevel verbose
It will not work.

Kureev · 2017-10-16T13:18:20Z

Detox core now only supports env vars, not argv. Try running
node_modules/.bin/mocha e2e --opts e2e/mocha.opts --configuration ios.sim.release --loglevel verbose
It will not work.

Sure, but this is not about Detox either. Detox should be run only by detox-cli and detox test command, isn't it correct?

rotemmiz · 2017-10-16T13:27:26Z

I don't agree, Detox is test runner independent, and we want to keep it that way.
What if people are using AVAJS now? They have no way of using detox-cli, and we're changing their API under their feet just to add support for other test runner in our CLI tool...
That will not be responsible of us.

In any case, using argv (where possible) is a better and responsible solution, it removes any possibility of environment var leaks.

Kureev · 2017-10-16T13:35:57Z

What if people are using AVAJS now?

I expect it to be not supported (yet). Otherwise I would expect an article in docs and/or other guidelines' describing how to setup Detox with your-faviourite-tool.

In any case, using argv (where possible) is a better and responsible solution, it removes any possibility of environment var leaks.

I'm fine changing code to support argv on argsparse level, but the part about env var leaks is still not clear to me.

Kureev · 2017-10-16T15:01:48Z

Should be good now. First we look for the flag in argv and if it isn't there (we use Jest), it falls back to env variable

rotemmiz · 2017-10-16T22:08:39Z

detox/test/ios/example.xcodeproj/project.pbxproj

@@ -975,7 +975,7 @@
 			buildSettings = {
 				ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon;
 				DEAD_CODE_STRIPPING = NO;
-				DEVELOPMENT_TEAM = "";
+				DEVELOPMENT_TEAM = J966JLDEF9;
 				INFOPLIST_FILE = example/Info.plist;
 				IPHONEOS_DEPLOYMENT_TARGET = 9.0;
 				LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks";


Was that added in purpose?

Kureev · 2017-10-17T00:41:18Z

I reset example.xcodeproj (sorry, committed this change by mistake. I was running this code on my emulator by hand, so had to sign it with certificate ¯\_(ツ)_/¯) + it seems that tests are green again!

Kureev · 2017-10-17T01:10:38Z

The last CI run was flaky 😒 I changed nothing (okok, I removed my cert pub hash from xcodeproj), but have failed specs. Can you please try to re-run this and let me know if there is anything else you would like me to take care of?

rotemmiz · 2017-10-17T17:42:06Z

detox/local-cli/detox-test.js

+      loglevel: program.loglevel,
+      cleanup: program.cleanup,
+      reuse: program.reuse,
+      debugSynchronization: program.debugSynchronization ? 3000 : '',


@Kureev , wouldn't this override every number value with 3000 ?

Hey @rotemmiz! Nice catch, fixed! 😉

rotemmiz · 2017-10-17T19:21:55Z

🎉 !!!
Thanks @Kureev !!

xfumihiro · 2017-10-17T19:38:15Z

@hellogerard Jest's TestEnvironment is sandboxed (see this)
Here's the PR for Global setup/teardown

Alexey added 2 commits October 11, 2017 14:04

Add basic jest runner support

d5d93cc

Adjust Jest documentation

ba08fba

rotemmiz reviewed Oct 11, 2017

View reviewed changes

Add missing --runInBand flag to jest command

8bf5abd

rotemmiz mentioned this pull request Oct 11, 2017

Android feedback #334

Closed

rotemmiz mentioned this pull request Oct 13, 2017

Detox core should not read --configuration name from argv directly #338

Closed

Alexey and others added 4 commits October 13, 2017 14:33

Use process.env instead of process.argv (not supported in Jest)

bcb0174

Merge branch 'master' into feature/support-jest-runner

9157bb0

Adjust tests to the new env approach

efe3b39

Adjust indentation in package.json

c3c0077

Add process.env to child process

92a1c49

Support runner option in detox config

14d6965

Kureev added 3 commits October 16, 2017 00:37

Update Guide.Jest to document a new way of changing runner

256af2b

Revert mocha.opts change (not a part of the scope)

afcfc0e

Revert formatting changes (not a part of the scope)

8fde423

Alexey added 2 commits October 16, 2017 16:11

Use alternative flow to handle env variables

8c9dc3b

Support argv and priorotize them over env vars

8092625

Cleanup detox-test.js

dc45bc2

rotemmiz reviewed Oct 16, 2017

View reviewed changes

Kureev added 3 commits October 17, 2017 01:15

Setup debug-synchronization value

63b4519

Fix runner problem

35e0f65

Reset example.xcodeproj

3803b47

Merge branch 'master' into feature/support-jest-runner

21690b8

rotemmiz reviewed Oct 17, 2017

View reviewed changes

Remove constant override

5fcfa4a

rotemmiz merged commit 2647b68 into wix:master Oct 17, 2017

Kureev deleted the feature/support-jest-runner branch October 18, 2017 07:04

wix locked and limited conversation to collaborators Jul 23, 2018

Basic support for Jest runner #335

Basic support for Jest runner #335

Conversation

Kureev commented Oct 11, 2017

rotemmiz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kureev Oct 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kureev Oct 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielMSchmidt commented Oct 11, 2017

hellogerard commented Oct 11, 2017 • edited Loading

Kureev commented Oct 11, 2017 • edited Loading

rotemmiz commented Oct 11, 2017

rotemmiz commented Oct 11, 2017

hellogerard commented Oct 12, 2017

Kureev commented Oct 13, 2017 • edited Loading

hellogerard commented Oct 13, 2017 • edited Loading

Kureev commented Oct 14, 2017

rotemmiz commented Oct 14, 2017

Kureev commented Oct 14, 2017

rotemmiz commented Oct 14, 2017

Kureev commented Oct 14, 2017

Kureev commented Oct 14, 2017 • edited Loading

rotemmiz commented Oct 14, 2017

Kureev commented Oct 14, 2017

Kureev commented Oct 14, 2017 • edited Loading

rotemmiz commented Oct 14, 2017

rotemmiz commented Oct 14, 2017

Kureev commented Oct 14, 2017

Kureev commented Oct 15, 2017

rotemmiz commented Oct 16, 2017

Kureev commented Oct 16, 2017

rotemmiz commented Oct 16, 2017

Kureev commented Oct 16, 2017

rotemmiz commented Oct 16, 2017 • edited Loading

Kureev commented Oct 16, 2017

Kureev commented Oct 16, 2017

Choose a reason for hiding this comment

Kureev commented Oct 17, 2017

Kureev commented Oct 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rotemmiz commented Oct 17, 2017

xfumihiro commented Oct 17, 2017

Kureev Oct 11, 2017 •

edited

Loading

Kureev Oct 11, 2017 •

edited

Loading

hellogerard commented Oct 11, 2017 •

edited

Loading

Kureev commented Oct 11, 2017 •

edited

Loading

Kureev commented Oct 13, 2017 •

edited

Loading

hellogerard commented Oct 13, 2017 •

edited

Loading

Kureev commented Oct 14, 2017 •

edited

Loading

Kureev commented Oct 14, 2017 •

edited

Loading

rotemmiz commented Oct 16, 2017 •

edited

Loading